-
-
Notifications
You must be signed in to change notification settings - Fork 11
Add nomination issue template #23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Adds links to contact options when creating a new issue. https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/configuring-issue-templates-for-your-repository#configuring-the-template-chooser
A new, more organized way to nominate Nixpkgs committers. https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/configuring-issue-templates-for-your-repository#creating-issue-templates
|
Will want to set up CI to notify #50 |
wolfgangwalther
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, there were some commands that people are running nowadays to get the respective links to "reviewed PRs", "authored PRs" etc.
Maybe provide these commands in the PR template as a base? Or would these be filled by CI somehow?
| - type: markdown | ||
| attributes: | ||
| value: | | ||
| Thanks for taking the time to fill out this bug report! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bug report is wrong, I think.
SigmaSquadron
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do apologise if we shouldn't review this yet, as it's a draft, but there are some things I learned while making the YAML forms for Nixpkgs that may be helpful here too.
| @@ -0,0 +1,25 @@ | |||
| name: Committer Nomination | |||
| description: Nominate a user for Nixpkgs commit access. | |||
| title: "[Nomination]: " | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| title: "[Nomination]: " | |
| title: "[Nomination]: NAME" |
We have placeholders like PACKAGENAME in Nixpkgs, and they usually work like 95% of the time. Since the only people being nominated here are supposed to be reasonably proficient with Nixpkgs' workflow, I think everyone will manage to replace it.
| @@ -0,0 +1,25 @@ | |||
| name: Committer Nomination | |||
| description: Nominate a user for Nixpkgs commit access. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This wording seems to imply that self-nominations will no longer be allowed? Will there be a separate issue template for self-nominations? Are they outright banned?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get that from the wording, just says you're nominating a singular user.
| labels: ["nomination"] | ||
| assignees: | ||
| - @NixOS/commit-bit-delegation | ||
| body: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would highly recommend adding a markdown item as the first form component that explains in more detail what this is all for. See the Nixpkgs bug report template for reference.
| label: GitHub user name | ||
| placeholder: octocat |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| label: GitHub user name | |
| placeholder: octocat | |
| label: GitHub Username | |
| description: The GitHub username currently used by the committer nominee. |
In Nixpkgs, we removed placeholders in order to reduce the overall information density. I would recommend doing the same here and using descriptions instead.
| - type: textarea | ||
| id: reason | ||
| attributes: | ||
| label: Reason | ||
| description: Why I are you nominating this person? Please include links to their relevant PR and reviewing activity. | ||
| validations: | ||
| required: true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like we can split this up into a few more questions. Most nominations in the old issue follow mostly the same format (sometimes the first and second paragraph are flipped):
- First paragraph introduces the person and shows something notable that they have done. This is usually a large NixOS module or something equally impactful.
- Second paragraph usually mentions how long the person has been contributing for, and what their merged/reviewed PR stats are. (unless the stats are at the bottom of the post)
- Third paragraph is usually the actual reason why the person should be a committer.
I think splitting this question appropriately will allow the delegation team to more efficiently review nominations, as they would have a very similar format to each other.
| - name: Contact the Committer Delegation Team via group message on Discourse | ||
| url: https://discourse.nixos.org/g/nixpkgs-nominations | ||
| about: For private inquiries and suggestions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This feels a bit redundant since the Discourse link is also available from the documentation below.
| id: reason | ||
| attributes: | ||
| label: Reason | ||
| description: Why I are you nominating this person? Please include links to their relevant PR and reviewing activity. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| description: Why I are you nominating this person? Please include links to their relevant PR and reviewing activity. | |
| description: Why are you nominating this person? Please include links to their relevant PR and reviewing activity. |
|
NixOS/nixpkgs#368656 is the PR (significant!) that @SigmaSquadron authored to successfully convert over Nixpkgs issues to templates. His feedback is really informed and wise! I want to say a few things with this PR:
|
|
I'd like to help out with getting this across the finish line, but I believe this should actually be done using PRs instead of issues, because
While PR templates don't support the same interactive forms that issue templates do, I don't think we benefit a lot from such issue templates for this anyways, because it's mostly free-form, and any more guided questions we might want to ask don't make a lot of sense to ask for non-self-nominations (e.g. introduce yourself, what do you plan to use commit access for, what are your conflicts of interest, ...). While there would be a small annoyance if the committer addition would have to be entered manually because of #31, I just opened NixOS/nixpkgs#33 to avoid having to do that. This then makes it as simple as linking to https://github.com/NixOS/nixpkgs-committers/new/main/members?filename=GITHUB_HANDLE for nominations. |
For me, point 2 trumps every other one, since it makes the acceptance process both transparent and asynchronous. Since issues cannot be accepted, only commented on, the UI is just worse by default. I do note that we could hack issues to get what we want, primarily with labels, so that each member of the commit bit team gets their own "✅" and "❌" label they can use on the issue. While it's not as clear as the acceptance from a PR, it's not a complete blocker either. The downside, in my view, is that most of the "stuff" for a PR -- the code most especially -- isn't relevant for the nomination. So there's a bunch of functionality that has be ignored. |
Maybe we can kill two birds with one stone here, and provide a small tool for getting these basic stats, opening a PR creating the relevant file, and auto-filling the title and body. Presumably, this would be simplest as a small wrapper around the I'm imagining something like |
|
I see no need to distribute such a niche tool with nixpkgs when |
|
(I'm sure it was just an example! Obviously it can live here. :)) I have thoughts about this, I'll take some time to type them up this weekend. |
|
I'd rather not introduce tools for something like this, that seems like over-engineering. If we want to get contributor stats automatically we can just have a GitHub action workflow run them and post it as a comment. |
|
I'm proposing a PR-based workflow here as an alternative: #34 |
|
Superseded by NixOS/nixpkgs#34. |
A new, more organized way to nominate Nixpkgs committers.
https://docs.github.com/en/communities/using-templates-to-encourage-useful-issues-and-pull-requests/configuring-issue-templates-for-your-repository#creating-issue-templates